Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use transposed size after opening for TIFF images #8390

Merged
merged 5 commits into from
Sep 18, 2024

Conversation

radarhere
Copy link
Member

Alternative to #8387

At the moment, load() might change the size of a TIFF image, since TIFF images are automatically transposed when doing so.

load() is a method that many users are probably not even aware of, and so they might find

from PIL import Image
im = Image.open("/Users/andrewmurray/pillow/Pillow/Tests/images/g4_orientation_5.tif")
print(im.size)  # (88, 590)
print(im.copy().size)  # (590, 88)

unexpected.

After this PR, the first im.size will also be (590, 88). This has means that changes from #6186 and #6190 can be removed.

#8387 solves this by making im.size sometimes different to im._size. I would like to avoid that, and have instead found a solution by adding TiffImageFile.load_prepare().

While we are here streamlining things so that an image does not change its size after loading, I've also updated ImageFile to remove an implication that loading might change an image's mode.

@@ -322,7 +322,7 @@ def load(self) -> Image.core.PixelAccess | None:

def load_prepare(self) -> None:
# create image memory if necessary
if self._im is None or self.im.mode != self.mode or self.im.size != self.size:
Copy link
Member

@homm homm Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how it affects multipage images with different page sizes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When seeking to a different TIFF frame, self.im is populated afresh.

def seek(self, frame: int) -> None:
"""Select a given frame as current image"""
if not self._seek_check(frame):
return
self._seek(frame)
# Create a new core image object on second and
# subsequent frames in the image. Image may be
# different size/mode.
Image._decompression_bomb_check(self.size)
self.im = Image.core.new(self.mode, self.size)

Copy link
Member

@homm homm Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I saw this and tried to remove this initialization and relay on load_prepare instead, for some reasons it didn't work.

For me providing correct canvas (self.im) in the base class is more preferable than recreating it in the plugin. There was another approach which doesn't introduce difference between im.size and im._size.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see now - my load_prepare() would create a new core image when the image was loaded after seek().

I've pushed a commit following your other approach.

src/PIL/TiffImagePlugin.py Outdated Show resolved Hide resolved
src/PIL/TiffImagePlugin.py Show resolved Hide resolved
@radarhere radarhere force-pushed the tiff_exif_transpose branch 2 times, most recently from 3ab41ec to a92dca6 Compare September 18, 2024 12:47
@hugovk hugovk merged commit 1ee3bd1 into python-pillow:main Sep 18, 2024
50 checks passed
@radarhere radarhere deleted the tiff_exif_transpose branch September 18, 2024 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants